How does this work when that input and output node-types are distinct? My thinking is:

• If no rule applies, we can't just return the node as it is, because it has the wrong type (the node kind is not valid in the output node-types)
• A rule cannot apply multiple times, because the output AST cannot generally be matched by a query that operates on the input AST.
  • If a given node name appears in both the input and output ASTs, but means something completely different in the two ASTs, will we accidentally re-apply rules on the output node? (misinterpreting it to be an input node of that kind)

The short answer is: it doesn't. The behaviour described where unknown nodes are passed through without modification only applies to the case where the input language is a subset of the output language.

As for your second point, I think it should work to match against both input and output types -- I'll add a test to make sure. The original intent was that one could have two phases of desugaring: the first one turns the input AST into a nicer output AST (this is the part we care more about right now), the second desugars complex constructs into simpler one (this is the part we focused on in the hackathon).

However, there is one subtlety that we might want to address: currently we rewrite the parent node before we descend into its children (and if no rewrite applies, we don't go back to it later). This makes actually implementing the second phase awkward -- when we see an output node we want to desugar, its children will still be in the input AST. It would probably be better to use a traversal where child nodes are handled before their parent. (However, for simple cleanup transformations, or more generally for transformations where we don't inspect the structure of the child nodes, it doesn't make a difference.)

Finally, if the same node type appears both in the input and output node types, then yes, we could accidentally re-apply rules to the output node. However, merely having the same node type isn't enough. The entire query has to match. Thus, if we do something like, say, change the name of a field, the query will stop matching, and we won't loop.

However, there is (at least) one case where we would loop. Consider the rule

(foo (_) @children) => (foo bar: {..children})

(that is, moving all unnamed children into a field). In this case, we would match repeatedly. The first time around we move all of the children into the bar field. The second time around we can still match, capturing an empty list of children in @children, and then overwrite the bar field, and so on. This continues until we hit the recursion depth limit (currently 100).

To mitigate this, what we could do is enforce that a given rule is only applied once, either globally or on a rule-by-rule basis. For simple AST cleanup, I don't currently see any issues with enforcing this behaviour globally. For more advanced desugaring, it might be detrimental.

a0a0e9e demonstrates that output->output transformations are indeed possible.

I have a solution to the rule repetition issue. It essentially adds a .repeated() method to Rule, which simply sets a corresponding internal boolean value to true. If the boolean is false, the rule in question is not run again on a node where it was just applied. (Other rules are unaffected, even rules that would change the current node.)

To me, single-use (per node) rules seems like the better default -- for most rules (especially cleanup rules) it is sufficient.

I also have a separate change that introduces a notion of "phases" to the runner. This makes it possible to specify separate "cleanup" and "desugar" phases.

I can add the former fix to this PR, if you prefer. Now that the CI is finally passing, I would rather just merge it and fix these issues in a separate PR (which, with some luck, won't have to run all the CI checks, unlike this one).

Happy to merge it like this and iterate on it from here.

For our current use-case, I'd expect the output types to diverge completely from the input types (no overlap except for coincidental name clashes). I think in practice we'll want one rule per node kind, possibly with a few node kinds being split across multiple rules. But there's no need to do everything in one PR.